Stack: Support cap_add, cap_drop and privileged on services #1940

olljanat · 2019-06-11T20:21:06Z

- What I did
Add support for cap_add, cap_drop and privileged options in services with stack deploy

Fourth step to address moby/moby#25885

- How I did it

Bump moby and swarmkit (not to latest because it looks to contain breaking changes but just enough to get this one working).
Included logic wich converts cap_add, cap_drop and privileged to exact capabilities list

- How to verify it
Use stack file like this:

version: "3.7"
services:
  test:
    image: ollijanatuinen/capsh
    networks:
      - test
    cap_add:
      - NET_ADMIN
    cap_drop:
      - CAP_MKNOD

networks:
  test:
    driver: overlay

or this

version: "3.7"
services:
  test:
    image: ollijanatuinen/capsh
    networks:
      - test
    privileged: true

networks:
  test:
    driver: overlay

Check log with command: docker logs <container id> which should print capabilities from inside of container.

- A picture of a cute animal (not mandatory but encouraged)

olljanat · 2019-06-11T20:57:17Z

Looks that those moby / swarmkit bumps generates need to modify existing code so will split them to another PR later (if no one else don't do that before me).

mterron · 2019-06-24T01:09:16Z

Why is this syntax different form the docker-compose syntax? It makes using swarm difficult. We already have a syntax for adding and removing capabilities, can't you use that?

olljanat · 2019-06-24T18:21:44Z

@mterron very valid point. I can investigate that option too. Whole CLI solution is currently bit open that how it should be implemented so all feedback is welcome (discussion is going on moby/moby#25885 (comment) )

PS. Marked this PR to WIP

codecov-io · 2019-09-10T18:28:37Z

Codecov Report

Merging #1940 into master will increase coverage by 0.02%.
The diff coverage is 83.33%.

@@            Coverage Diff             @@
##           master    #1940      +/-   ##
==========================================
+ Coverage   56.78%   56.81%   +0.02%     
==========================================
  Files         311      311              
  Lines       21839    21863      +24     
==========================================
+ Hits        12402    12422      +20     
- Misses       8522     8524       +2     
- Partials      915      917       +2

codecov-io · 2019-09-10T18:28:37Z

Codecov Report

❗ No coverage uploaded for pull request base (master@4538b15). Click here to learn what that means.
The diff coverage is 83.33%.

@@            Coverage Diff            @@
##             master    #1940   +/-   ##
=========================================
  Coverage          ?   56.81%           
=========================================
  Files             ?      311           
  Lines             ?    21863           
  Branches          ?        0           
=========================================
  Hits              ?    12422           
  Misses            ?     8524           
  Partials          ?      917

olljanat · 2019-09-10T18:39:20Z

@mterron now this works with existing cap_add/cap_drop/privileged syntax. Can you test?

@thaJeztah PTAL

mterron · 2019-09-12T23:46:53Z

Looks good.

westsouthnight · 2019-10-08T22:46:21Z

Somebody up merge

nurettin · 2019-10-30T05:09:55Z

Is this the roadmap for this feature?
moby/moby#25885 (comment)

kolyshkin · 2019-10-31T17:49:04Z

@olljanat this needs a rebase

olljanat · 2019-10-31T19:03:56Z

@kolyshkin for vendor bump yes but can you plz review the actual implementation first?

olljanat · 2019-11-14T18:50:50Z

Rebased. Default capabilities are duplicated here because DefaultCapabilities() -function from github.com/docker/docker/oci cannot be vendored because it would include some other references which does not pass cross compile.

Created moby/moby#40212 which fixes that issue.

olljanat · 2019-11-27T19:13:16Z

@Zerocjx sounds that you are not updated Docker engine (dockerd) to master build. If that does not help plz ping me on Docker community Slack (as that sounds more like usage issue than bug).

@cpuguy83 @alexellis Added now also check for API version because those parameters exists already on earlier engine versions but only API version >= 1.40 support them with stack which would be very confusing for users. Also added requested tests.

@cpuguy83 Starting container without no capabilities at all should works just like on docker run / docker-compose commands where user need specify all default ones to cap_droplist. Another options would be introduce some special capability e.g. NONE (like there actually is ALL already). However I'm not sure if there actually is any real world use case for that scenario? And even if there is probably it would be better to implement it separately?

zeroxer · 2019-11-29T02:14:40Z

@olljanat thx. After replace the dockerd with the master dockerd build, it works.

aamkye · 2019-12-17T13:18:58Z

Hi, what is the status of cap_add and cap_drop support in docker stack?

olljanat · 2020-01-02T11:04:05Z

@cpuguy83 @alexellis (and maybe @thaJeztah too? ) please review again after latest changes.

cpuguy83

LGTM

Signed-off-by: Olli Janatuinen <[email protected]>

olljanat · 2020-02-16T12:36:37Z

Rebased. @thaJeztah @silvin-lubecki PTAL

olljanat · 2020-04-28T06:42:18Z

ping @thaJeztah @silvin-lubecki

tuxity · 2020-05-07T20:49:20Z

Oh! Exactly what I'm looking for!
I use GDB inside of my docker containers normally and I'm trying to migrate to Swarm, but GDB need STRACE cap to start

How I can test it @olljanat ? I guess I need to manually install this version of CLI and maybe dockerd ?
EDIT: nvm info is in some comments a bit higher :D

olljanat · 2020-05-24T08:15:11Z

@tiborvass I can see that you have merged some PRs to here lately. Maybe you can check this one too?

contrem · 2020-06-13T19:36:20Z

Please confirm:

A config like the following results in all capabilities instead of no capabilities:

version: "3.8"
services:
    test:
        image: *
        cap_drop:
            - ALL

The capabilities array comes back as empty in service.go line 121.

Which I think is interpreted the same as not specifying any caps (defaulting to all caps) in both the swarmkit pb file and docker api container.go because it's empty. They can't distinguish between not specifying caps (no caps array) and drop all caps (empty caps array).

A special NONE value may be required.

olljanat · 2020-06-13T20:14:44Z

@contrem a lot of time has passed after I implemented those so I cannot say sure but my supposition is that "ALL" should invalid value for cap_add/cap_drop on Moby side and give error. If it does not work like that then it is most probably bug on Moby.

If there is some real world use case for special NONE capability IMO it should be implemented as separate PR on moby side as this is part of trying get same logic to docker service which have been years available with docker-compose.

cbrherms · 2020-06-13T20:34:38Z

I'm surprised after all this time this has still yet to be merged.

contrem · 2020-06-13T20:45:35Z

The ALL value is handled in https://github.com/moby/moby/blob/master/oci/caps/utils.go#L120

The behavior I expected would be the same as

docker run -it --cap-drop all ollijanatuinen/capsh:

Current: =
Bounding set =
Securebits: 00/0x0/1'b0
 secure-noroot: no (unlocked)
 secure-no-suid-fixup: no (unlocked)
 secure-keep-caps: no (unlocked)
uid=0(root)
gid=0(root)
groups=0(root),1(bin),2(daemon),3(sys),4(adm),6(disk),10(wheel),11(floppy),20(dialout),26(tape),27(video)

Instead, the result using the previous config with image ollijanatuinen/capsh and docker stack deploy is:

Current: = cap_chown,cap_dac_override,cap_fowner,cap_fsetid,cap_kill,cap_setgid,cap_setuid,cap_setpcap,cap_net_bind_service,cap_net_raw,cap_sys_chroot,cap_mknod,cap_audit_write,cap_setfcap+eip
Bounding set =cap_chown,cap_dac_override,cap_fowner,cap_fsetid,cap_kill,cap_setgid,cap_setuid,cap_setpcap,cap_net_bind_service,cap_net_raw,cap_sys_chroot,cap_mknod,cap_audit_write,cap_setfcap
Securebits: 00/0x0/1'b0
 secure-noroot: no (unlocked)
 secure-no-suid-fixup: no (unlocked)
 secure-keep-caps: no (unlocked)
uid=0(root)
gid=0(root)
groups=0(root),1(bin),2(daemon),3(sys),4(adm),6(disk),10(wheel),11(floppy),20(dialout),26(tape),27(video)

When adding a single cap and dropping all others, it works as expected because the cap array returned is not empty:

version: "3.8"
services:
    test:
        image: ollijanatuinen/capsh
        cap_add:
            - chown
        cap_drop:
            - all

Current: = cap_chown+eip
Bounding set =cap_chown
Securebits: 00/0x0/1'b0
 secure-noroot: no (unlocked)
 secure-no-suid-fixup: no (unlocked)
 secure-keep-caps: no (unlocked)
uid=0(root)
gid=0(root)
groups=0(root),1(bin),2(daemon),3(sys),4(adm),6(disk),10(wheel),11(floppy),20(dialout),26(tape),27(video)

I don't know where the empty cap array issue should be handled, but I do think the cap_drop all handling is unexpected behavior.

oramirite · 2020-06-19T23:32:23Z

I am cheering for you all SO HARD on this one. This is such a crucial feature and will unlock so many use-cases for us. Thank you @olljanat for your work championing this feature and I hope we see this in the next release.

tu-nv · 2020-06-27T12:14:38Z

While already 👍 and ❤️ , my inner still want to post this useless comment to show that I (and many others) really hope this feature will be available soon. I will go back to cheer with many 🎉🎉🎉🎉🎉 when it is finally merged.

georgmay · 2020-07-01T16:15:02Z

I can't believe it's still not merged...

cpuguy83 · 2020-07-06T16:50:21Z

I think we are going to have to revert these API changes in the engine and swarmkit since the API does not seem to hold up well. This is based on discussions in the maintainers call last Thursday.

Specifically some of the things mentioned early on in the design review here are problematic.
Namely that the client must specify the full list of caps, meanwhile the client does not know actually what caps are available on the engine host.

cpuguy83 · 2020-07-23T18:12:13Z

moby/moby#41249 makes the neccessary changes to moby's API for correcting cap add/drop support.

akerouanton · 2020-07-28T16:08:20Z

@olljanat Do you need some help to finish this PR? 🙂

olljanat · 2020-07-28T16:26:19Z

@akerouanton if you or someone else want take responsibility to finalize this I'm more than happy to give it forward. I did see than PR on moby side was merged but have not found time to investigate what actually is need to be changed here.

thaJeztah · 2020-07-28T17:53:44Z

I'm working on updating the vendored version of moby in #2659 (opened as draft as it may need local changes). After that it should be possible to implement the changes here to wire that up

GordonTheTurtle added the status/0-triage label Jun 11, 2019

olljanat mentioned this pull request Jun 11, 2019

Missing from Swarmmode --cap-add moby/moby#25885

Closed

olljanat force-pushed the service-capabilities-support-stack branch 2 times, most recently from 2d8b1a3 to b1bf3ee Compare June 11, 2019 20:44

olljanat changed the title ~~Capabilities support for stack~~ WIP: Capabilities support for stack Jun 24, 2019

olljanat force-pushed the service-capabilities-support-stack branch from b1bf3ee to 88c03ea Compare September 9, 2019 18:20

olljanat changed the title ~~WIP: Capabilities support for stack~~ Stack: Support cap_add, cap_drop and privileged on services Sep 9, 2019

olljanat force-pushed the service-capabilities-support-stack branch 4 times, most recently from 2e26f86 to 9fab7bd Compare September 10, 2019 18:22

duncan-brown added a commit to duncan-brown/ce-it-infrastructure that referenced this pull request Oct 27, 2019

use compose until docker/cli#1940 is available

5eaf4de

duncan-brown added a commit to cosmic-explorer/ce-it-infrastructure that referenced this pull request Oct 27, 2019

use compose until docker/cli#1940 is available

e4290d0

olljanat force-pushed the service-capabilities-support-stack branch 4 times, most recently from 26791a9 to 41bd3ea Compare November 14, 2019 17:50

olljanat mentioned this pull request Nov 14, 2019

Move DefaultCapabilities() to caps package moby/moby#40212

Merged

olljanat force-pushed the service-capabilities-support-stack branch from ed8d5f3 to c38ab47 Compare November 27, 2019 18:57

rmartinsanta mentioned this pull request Dec 11, 2019

Docker Swarm incompatiblity DOMjudge/domjudge-packaging#48

Closed

cpuguy83 approved these changes Jan 2, 2020

View reviewed changes

Stack: Support cap_add, cap_drop and privileged on services

ce5d7c5

Signed-off-by: Olli Janatuinen <[email protected]>

olljanat force-pushed the service-capabilities-support-stack branch from c38ab47 to ce5d7c5 Compare February 16, 2020 10:46

Re-vendor Moby

fa80a15

Signed-off-by: Olli Janatuinen <[email protected]>

olljanat force-pushed the service-capabilities-support-stack branch from 7d0c25a to fa80a15 Compare February 16, 2020 11:08

akerouanton mentioned this pull request Jul 30, 2020

Add capabilities support to stack/service commands #2663

Closed

olljanat closed this Aug 24, 2020

thaJeztah mentioned this pull request Aug 25, 2020

[carry 2663] Add capabilities support to stack/service commands #2687

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stack: Support cap_add, cap_drop and privileged on services #1940

Stack: Support cap_add, cap_drop and privileged on services #1940

olljanat commented Jun 11, 2019 •

edited

Loading

olljanat commented Jun 11, 2019

mterron commented Jun 24, 2019

olljanat commented Jun 24, 2019

codecov-io commented Sep 10, 2019

codecov-io commented Sep 10, 2019 •

edited

Loading

olljanat commented Sep 10, 2019

mterron commented Sep 12, 2019

westsouthnight commented Oct 8, 2019

nurettin commented Oct 30, 2019

kolyshkin commented Oct 31, 2019

olljanat commented Oct 31, 2019

olljanat commented Nov 14, 2019

olljanat commented Nov 27, 2019

zeroxer commented Nov 29, 2019

aamkye commented Dec 17, 2019

olljanat commented Jan 2, 2020

cpuguy83 left a comment

olljanat commented Feb 16, 2020

olljanat commented Apr 28, 2020

tuxity commented May 7, 2020 •

edited

Loading

olljanat commented May 24, 2020

contrem commented Jun 13, 2020

olljanat commented Jun 13, 2020

cbrherms commented Jun 13, 2020

contrem commented Jun 13, 2020

oramirite commented Jun 19, 2020

tu-nv commented Jun 27, 2020

georgmay commented Jul 1, 2020

cpuguy83 commented Jul 6, 2020

cpuguy83 commented Jul 23, 2020

akerouanton commented Jul 28, 2020

olljanat commented Jul 28, 2020

thaJeztah commented Jul 28, 2020

Stack: Support cap_add, cap_drop and privileged on services #1940

Stack: Support cap_add, cap_drop and privileged on services #1940

Conversation

olljanat commented Jun 11, 2019 • edited Loading

olljanat commented Jun 11, 2019

mterron commented Jun 24, 2019

olljanat commented Jun 24, 2019

codecov-io commented Sep 10, 2019

Codecov Report

codecov-io commented Sep 10, 2019 • edited Loading

Codecov Report

olljanat commented Sep 10, 2019

mterron commented Sep 12, 2019

westsouthnight commented Oct 8, 2019

nurettin commented Oct 30, 2019

kolyshkin commented Oct 31, 2019

olljanat commented Oct 31, 2019

olljanat commented Nov 14, 2019

olljanat commented Nov 27, 2019

zeroxer commented Nov 29, 2019

aamkye commented Dec 17, 2019

olljanat commented Jan 2, 2020

cpuguy83 left a comment

Choose a reason for hiding this comment

olljanat commented Feb 16, 2020

olljanat commented Apr 28, 2020

tuxity commented May 7, 2020 • edited Loading

olljanat commented May 24, 2020

contrem commented Jun 13, 2020

olljanat commented Jun 13, 2020

cbrherms commented Jun 13, 2020

contrem commented Jun 13, 2020

oramirite commented Jun 19, 2020

tu-nv commented Jun 27, 2020

georgmay commented Jul 1, 2020

cpuguy83 commented Jul 6, 2020

cpuguy83 commented Jul 23, 2020

akerouanton commented Jul 28, 2020

olljanat commented Jul 28, 2020

thaJeztah commented Jul 28, 2020

olljanat commented Jun 11, 2019 •

edited

Loading

codecov-io commented Sep 10, 2019 •

edited

Loading

tuxity commented May 7, 2020 •

edited

Loading